Hi @joslloand
I haven't looked at this closely, but wouldn't it be possible to have this value set from sclang?

I haven't looked at this closely, but wouldn't it be possible to have this value set from sclang?
@dyfer, ah, that might be better, as it would then be changeable?

Ah, this is a nice idea!

In practice, I don't think I'd want this to be an argument that the user supplies directly in the user interface to the UGen. Doing so could / would break existing code.

Instead, it should happen "under the hood" by setting AtkFoa.speedOfSound to the relevant value.

Ok, having had a little look, the changes to do so would have to happen in both AtkUGens.cpp here in sc3-plugins, and in ATK.sc found in the quark.

@joshpar, what do you think of this?

And.. I just noticed that, as we actually need normalized frequency in the calculation:

Could be updated to:

float wc = speedOfSound / distanceStart * SAMPLEDUR;

I'm feeling this would make things more simple, and clearer.

Instead, it should happen "under the hood" by setting AtkFoa.speedOfSound to the relevant value.

Yes, that's what I was thinking

Instead, it should happen "under the hood" by setting AtkFoa.speedOfSound to the relevant value.

Yes, that's what I was thinking

Good to know!

The slight danger is that users will need to make sure to update both sc3-plugins and the atk-sc3 quark. That shouldn't be a problem if the change is communicated in the update announcement..

@joshpar, how does this sound to you. (I'll add @mtmccrea, too.)

yeah @joslloand - I think that's a cool idea

yeah @joslloand - I think that's a cool idea

Ok, great!

As for actually making it happen, I'm super rusty on the plugin side and would need to have some guidance on getting it right.

In particular, once the speed of sound is set, it should be constant for the life of the UGen instantiation. I.e., it should be a fixed value that isn't modulate-able.

@dyfer, I may reach out over email for a quick chat on the matter....

I can help as well if needed. Cheers!

Ok, great! I think I see what needs to happen... so that's fine. (However, since, I haven't tried to compile sc3-plugins on this machine in so long, I'm expecting a task!)

Ok, I've just made an updated push.

The updated atk-sc3 quark branch is here: https://github.com/ambisonictoolkit/atk-sc3/tree/refactor-foa-nfc-proximity

What I haven't done is tried to compile... so testing is required....

@joslloand we've just added automated building with github actions. If you rebase your PR on the current main, you'll see an automated build in this PR, where you can also download the build artifacts to test. However, building locally is definitely recommended :)

@joslloand we've just added automated building with github actions. If you rebase your PR on the current main, you'll see an automated build in this PR, where you can also download the build artifacts to test. However, building locally is definitely recommended :)

Thanks @dyfer! I didn't realize this... very convenient. (I ended up doing a merge rather than a rebase.)

I've done some tests, and both HOA and FOA are now matched. Hurrah! So the updates are working now.

... so... I think we're ready for a merge

@joslloand we've just added automated building with github actions. If you rebase your PR on the current main, you'll see an automated build in this PR, where you can also download the build artifacts to test. However, building locally is definitely recommended :)

Thanks @dyfer! I didn't realize this... very convenient. (I ended up doing a merge rather than a rebase.)

I've done some tests, and both HOA and FOA are now matched. Hurrah! So the updates are working now.

... so... I think we're ready for a merge

@dyfer, @joshpar just picking this up again after some time away....

... I think this is all ready to go. The tests (matching NFE results w/ the HOA implementation) have passed.

So, I think this is ready to be closed and merged.

@joshpar would you also agree that it is ready to go?

@joshpar would you also agree that it is ready to go?

@telephon I had a conversation with @dyfer yesterday, regarding backwards compatibility and resulting behavior given installs with mismatched quark and sc3-plugins.

I have a possible solution I'll be testing today that should resolve performance issues if the class and plugin aren't aligned. (I expect a simple check on inputs will do the job.)

If this does work, I'll go ahead and push this change... then I think we'll really be ready to go!

@telephon I had a conversation with @dyfer yesterday, regarding backwards compatibility and resulting behavior given installs with mismatched quark and sc3-plugins.

I have a possible solution I'll be testing today that should resolve performance issues if the class and plugin aren't aligned. (I expect a simple check on inputs will do the job.)

If this does work, I'll go ahead and push this change... then I think we'll really be ready to go!

Ok, here's the fix: 8bc45e4

Which meets these cases, testing cases for matching and unmatching quark AND plugins:

1. matching: latest test quark (refactor-foa-nfc-proximity) AND latest test plugins (topic/atk-fix-speedOfSound)
   --> pass: speed of sound updated to 343 m/s

2. NOT matching: previous released quark (v5.0.3) AND latest test plugins (topic/atk-fix-speedOfSound)
   --> pass: speed of sound remains at 333 m/s [ plugins supply missing speed of sound ]

3. NOT matching: latest test quark (refactor-foa-nfc-proximity) AND previous plugins (v3.11.1)
   --> pass: speed of sound remains at 333 m/s [ plugins cutoff frequency sets equivalent speed of sound, value supplied by class is ignored ]

@joshpar, @dyfer, @telephon, so now I'm thinking we're ready to go with this.

(Finally!)

This seems to work as advertised, thank you!

@joshpar @telephon is it OK to leave that merge commit in?

@joshpar is your change request satisfied? :)

Thanks everyone!